Skip to content

[SQL] Add short-term backup retention immutability support - #34207

Open
svmicano wants to merge 6 commits into
Azure:devfrom
svmicano:svmicano/sql-str-immutability
Open

svmicano wants to merge 6 commits into
Azure:devfrom
svmicano:svmicano/sql-str-immutability

Conversation

@svmicano

@svmicano svmicano commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

🤖 PR Validation — ⚠️ Review suggested

Breaking Changes Tests
⚠️ None ️✔️ 130/130
⚠️AzureCLI-BreakingChangeTest
⚠️sql
rule cmd_name rule_message suggest_message
⚠️ 1006 - ParaAdd sql db str-policy set cmd sql db str-policy set added parameter lock_immutability
⚠️ 1006 - ParaAdd sql db str-policy set cmd sql db str-policy set added parameter yes
⚠️ 1008 - ParaPropAdd sql db str-policy set cmd sql db str-policy set update parameter diffbackup_hours: added property type=int
⚠️ 1008 - ParaPropAdd sql db str-policy set cmd sql db str-policy set update parameter retention_days: added property type=int
⚠️ 1009 - ParaPropRemove sql db str-policy set cmd sql db str-policy set update parameter retention_days: removed property required=True
⚠️ 1006 - ParaAdd sql midb short-term-retention-policy set cmd sql midb short-term-retention-policy set added parameter lock_immutability
⚠️ 1006 - ParaAdd sql midb short-term-retention-policy set cmd sql midb short-term-retention-policy set added parameter yes
⚠️ 1008 - ParaPropAdd sql midb short-term-retention-policy set cmd sql midb short-term-retention-policy set update parameter retention_days: added property type=int

Related command

  • az sql db str-policy set, show, and wait
  • az sql midb short-term-retention-policy set and show (live and restorable dropped databases)

Description
Add short-term backup immutability support using 2026-08-01-preview, with the final async SET result aligned with Azure PowerShell PR 30215.

  • Add nullable --lock-immutability and --yes/-y to both SET commands. Only true prompts, with an irreversible-action warning. Decline/non-interactive confirmation failure sends no update and emits no result; --yes bypasses confirmation.
  • Reject --lock-immutability with --deleted-time, including explicit false, before HTTP. Never serialize that property for dropped MI databases.
  • Surface the service's immutabilityStatus on SET/show for SQL Database and live/dropped MI databases, retaining flattened CLI output and nested systemData.
  • SQL Database retention remains optional for differential-only input; omitted settings are not serialized as null. MI retention remains required, including support for zero on dropped databases. SQL DB lock-only input is serializable but is rejected by the tested service; the corrected help example supplies explicit retention.
  • Keep the focused SDK-independent shared AAZ HTTP/polling implementation and existing registrations, cloud endpoints, authentication, URL escaping, --ids, wait and --no-wait. azure-mgmt-sql==4.0.0b22 is unchanged; a whole-module migration to the hybrid models in 5.0.0b2 is out of scope.

Final policy GET alignment (2026-10-13)

Commit 93e16b8f2253fbdccb1a8337cc2dca763d6cae83 removes the explicit final-state-via: location override from the shared STR PUT operation. For the Azure-AsyncOperation flow recorded for SQL Database and MI:

  1. PUT the policy.
  2. Poll async status until Succeeded.
  3. GET the original policy resource URL and return that policy.

This matches the PowerShell recordings, whose final policy GET belongs to the SET operation. It replaces the previous OperationResults GET; it does not add a second final read, synthesize Locked, retry until a desired immutability status, add fixed delays, or fill omitted request properties.

  • --no-wait still submits once, without polling or a final GET, and emits no result.
  • Failed/canceled operations propagate their error before any final GET. Errors from the final policy GET are also propagated.
  • Synchronous 200 responses without polling headers and Location-only responses retain standard ARM behavior: return their response without an additional policy GET. Unit tests cover these distinct paths; no unconditional readback is claimed.
  • SQL DB and MI locking scenarios now assert Locked on SET itself, as well as through independent policy GETs. All five affected scenarios were genuinely re-recorded live on 2026-10-13; response statuses and URLs were not hand-edited to pass playback.
  • Fixed the recording sanitizer to preserve non-JSON/non-object gateway responses instead of masking HTTP retry/error handling with a JSON parsing exception.
  • Isolated process-wide CLI session caches in unit-test setup/cleanup so temporary configuration directories do not contaminate later scenario tests. Help regression coverage tolerates normal line wrapping.

Wait-command compatibility

The earlier fix in 3333619a720b9deb77c784134f1e276500dda0c4 remains intact: az sql db str-policy wait --policy-name default is accepted and --policy-name remains required, matching the exact PR base cc010d2104b3ab0089c33fb7eac8816c260a11ac. A dedicated getter forwards and URL-escapes the supplied name; show/set and MI commands still use the default policy without a new public option. Regression coverage includes --ids, encoded names and missing-option rejection without HTTP. No compatibility-rule suppression or exception is requested.

Remaining service limitations and release checks

  • OperationResults can still report Disabled while the actual policy GET reports Locked. Earlier raw SQL DB recordings and 24 paired MI diagnostic reads over approximately 144 seconds demonstrated this backend inconsistency. The CLI now uses the standard final policy GET for the recorded async flow; this does not claim the backend endpoint is fixed.
  • New SQL DB policies may initially report Disabled before becoming Enabled. The new-DB locking test retains its 900-second readiness timeout and refuses to lock before Enabled; independent post-lock checks retain their 300-second timeout. A previous Oct13 initialization failure is not counted as passing. The refreshed new-DB locking recording passed.
  • The tested SQL DB backend rejects lock-only PUTs with InvalidParameterValue; including current retention succeeds. On a locked policy, retention updates with the lock flag omitted can be interpreted as unlocking. Help now documents explicit retention when locking and preserving --lock-immutability true when increasing retention. No automatic payload completion was introduced.
  • Confirm preview API rollout for intended regions/clouds and obtain maintainer approval before release. This PR remains a draft.

Testing Guide

Latest local result: 44 tests passed, zero failures/errors/skips (28 CLI/HTTP unit tests + 11 scenario safety tests + 5 recorded scenarios). Offline execution used this checkout's CLI/core/test SDK source with the installed CLI libraries on Python 3.14.6, an empty Azure configuration, live-target flags removed, socket connections blocked, and only the unrelated release-update connectivity probe disabled.

The new original-resource final-GET regressions failed against the previous Location override and passed after the fix. A combined-suite run also exposed and fixed the unit-test session-cache isolation issue; the complete combined suite then passed in 70.238 seconds.

SQL Database scenarios

Source: SQL Database STR tests.

Live-recorded scenario HTTP interactions Coverage and assertions
test_sql_db_short_term_retention 40 New randomized S0/Local DB after absence check. Retention/differential settings 7/24 → 6/12 → 5/12 → 5/24; explicit false sets 7 days without prompting. SET and independent GET assert retention/interval. No-wait sets 8 days, returns no output, and GET verifies 8/24. Tolerates unlocked Disabled/Enabled.
test_sql_db_short_term_retention_immutability 70 New DB; wait for Enabled; initialize 7/12. Decline/no-TTY cause no HTTP. Accepted confirmation locks at 7 days; SET returns Locked and GET independently verifies it. --yes increases to 8 days with SET and GET Locked. Explicit-lock decrease and false unlock return InvalidImmutableBackupRetentionPeriod and CannotRemoveLockedPitrImmutability. Differential-only no-wait preserves Locked at 8/24.
test_sql_db_existing_policy_immutability 14 Explicitly authorized disposable DB already Enabled. Locks with unchanged 7-day retention and preserves 24-hour differential interval; SET and GET report Locked. Rejects reduction to 6 days and unlocking. Repeated lock with no-wait has no output/prompt; independent GET remains Locked7/24. Policy-only transport guard; no create/delete.

All three refreshed SQL DB scenarios passed live on 2026-10-13 (new DBs in West US 2; policy-only in West Central US), and passed offline playback. The two new SQL DBs were retained by request. There are now eight retained SQL test databases across the earlier and current validation runs; they continue to incur charges. The policy-only run modifies only its expressly authorized retained test DB, not an original database.

SQL Managed Instance scenarios

Source: SQL MI STR tests.

Live-recorded scenario HTTP interactions Coverage and assertions
test_sql_managed_db_short_retention 46 New randomized DB on existing MI; live retention 7 → 14 → 7(false) → 10(no-wait). No confirmation for omitted/false lock. SET/GET retention checks; wait for backup readiness, then drop only its own DB. Dropped policy 10 → 7 → 6(no-wait). Both true and false lock flags with deleted-time fail locally without HTTP or prompt. Final policy GET after async SET also covers the timestamp-addressed dropped resource.
test_sql_managed_db_short_retention_immutability 53 New DB must report Enabled. Ordinary updates/false before locking; decline/no-TTY send no HTTP. Accepted lock at 7 and --yes increase to 8 both return SET Locked, with independent GET verification. Service rejects reduction/unlocking. After dropping only its own DB, policy remains Locked8; both dropped lock-flag values fail locally, and the service rejects dropped-retention reduction.

Both refreshed MI recordings passed live in West Central US on 2026-10-13 and offline playback. Earlier attempts encountered a non-JSON response and an interrupted backup-readiness wait; these are not counted as successful recordings and were superseded by complete passing captures. All MI test databases from these attempts were cleaned up, including failed-attempt databases. The seven original MI database names remain unchanged; locked dropped backups are not purged and remain for their retention period.

Offline regressions, audits and CI

  • 28 CLI/HTTP unit tests: confirmation accept/decline/no-TTY/yes; omitted/false lock; request omission and integer/Boolean serialization; dropped-MI true/false rejection, zero retention, timestamp addressing; all three statuses; flattened output and nested systemData; required wait policy-name compatibility; IDs and help. Polling coverage includes async-only/both headers for all three resource types, InProgress → Succeeded → original policy GET, synchronous200, Location-only, no-wait request counts, Failed/Canceled without final read, final-GET403/404 propagation, and actual CLI SET JSON output. Lock-only serialization coverage does not imply live support.
  • 11 scenario safety tests: six SQL DB tests for explicit targets, own-resource/policy-only write guards, keep-database behavior and bounded policy waits; five MI tests for target/write scope, signed-URL sanitization, non-JSON/non-object preservation and live/dropped policy waits.
  • Recording audit passed for all five captures: real target names/subscription IDs and credential markers absent; signed polling parameters redacted; unrelated dropped databases filtered. Every recorded completed async STR SET uses the policy GET, with five Locked SET results across the three locking scenarios. No STR OperationResults GET remains in these captures. No-wait SETs have no polling/final reads.
  • Write-scope audit and post-run inventory/policy checks passed: prior database inventories preserved, only the two approved SQL DB additions remain, other retained test policies unchanged, and all MI attempt DBs removed. No server, MI or resource-group write is permitted by the guards; original databases are untouched.
  • Targeted pycodestyle, git diff --check, and editor diagnostics passed. The shared STR operation module passed Pylint at 10.00/10. Full local SQL-module Pylint is not claimed because the installed runtime has compiled-only dependencies and previously reported unrelated findings.
  • Hosted main/full-test/breaking-change/lint/style checks passed for the prior head 3333619a.... Checks for this new head 93e16b8f2253fbdccb1a8337cc2dca763d6cae83 must run independently; earlier green checks or bot summaries are not validation of this commit.

Hosted pipeline gate: Requested /azp run for this head, but the Azure Pipelines bot reported insufficient commenter privileges. A maintainer must trigger the main/full-test/breaking-change pipelines. The independent azdev-linter/style workflows are running/queued; their results are pending.

Replay with python -m unittest azure.cli.command_modules.sql.tests.latest.test_sql_str_policy azure.cli.command_modules.sql.tests.latest.test_sql_mi_str_policy azure.cli.command_modules.sql.tests.latest.test_sql_db_str_policy in a configured developer checkout.

Live recording requirements and safety guards: SQL STR recording guidance.

  • MI: AZURE_CLI_TEST_SQL_MI_RESOURCE_ID; irreversible locking additionally requires AZURE_CLI_TEST_SQL_MI_ALLOW_IMMUTABILITY_LOCK=true.
  • New SQL DB: AZURE_CLI_TEST_SQL_SERVER_RESOURCE_ID; locking additionally requires AZURE_CLI_TEST_SQL_DB_ALLOW_IMMUTABILITY_LOCK=true. These runs used AZURE_CLI_TEST_SQL_DB_KEEP_DATABASES=true.
  • Policy-only SQL DB: AZURE_CLI_TEST_SQL_DB_RESOURCE_ID plus separate irreversible-lock authorization. Requires Enabled and retention above one day; always retains the database.
  • Missing recordings do not authorize live access. Do not run the entire SQL module live on shared resources. Local diagnostic cassettes are excluded from this PR.

History Notes
[SQL] az sql db str-policy: Add short-term backup immutability locking with confirmation and expose immutability status
[SQL] az sql midb short-term-retention-policy: Add immutability locking for live databases and expose status for live and dropped databases


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@a0x1ab

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@yonzhan

Copy link
Copy Markdown
Collaborator

Please fix CI issues

@svmicano

Copy link
Copy Markdown
Member Author

Production MI validation — 2026-10-12

Validated commit 3fe125128615c9c7a7958520ccb10a970fa9be22 by running the edited CLI/core source against the public Azure production ARM endpoint in West Europe. Two new, uniquely named databases were created on an existing test MI. No MI-level write requests were made, and pre-existing databases were not modified.

Check Result
Preview STR show for both live databases Passed; retentionDays: 7, immutabilityStatus: Disabled initially
Synchronous retention update with lock flag omitted Passed on both databases: 7 → 14 days, verified by show
Explicit --lock-immutability false without --yes Passed without prompting: 14 → 7 days
Live --no-wait Passed: update returned without a result, then read-only polling/show confirmed 10 days
Declined confirmation / no-TTY safeguards Passed using a controlled prompt host: exit 1, no result, zero HTTP requests; live readback was unchanged
Authorized --lock-immutability true --yes Service rejected with LockPitrImmutabilityNotAllowed; readback remained Disabled, with retention unchanged
Dropped database STR show Passed after confirming backup readiness and dropping only a new test database
Dropped STR synchronous update Passed: 10 → 7 days, verified by show
Dropped STR --no-wait Passed: 7 → 6 days, verified by read-only polling/show
Dropped --lock-immutability true and false Both rejected locally: exit 1, no result, zero HTTP requests, no prompt
Cleanup / scope safety Both newly created databases deleted; MI configuration matched the initial baseline, and the two original databases remained online

The dropped test backups remain under their configured retention; they were not purged. Use the original textual deletionDate from az sql midb list-deleted (for example with --query ... --output tsv) so shell date conversion does not lose fractional seconds.

Still blocked: this MI reports immutability Disabled, and the service explicitly disallows locking. Successful locking, Locked readback and rejection of retention decreases after locking therefore remain unvalidated on live resources. No MI configuration was changed to work around this limitation. SQL Database (non-MI) live validation was not performed in this run.

These were live smoke checks, not newly captured/re-recorded scenario cassettes. Existing recording refresh and standard full-module validation remain outstanding; keep this PR in draft. No implementation changes were required by the supported-path checks.

@a0x1ab

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@a0x1ab

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@a0x1ab

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@svmicano

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 34207 in repo Azure/azure-cli

@svmicano
svmicano marked this pull request as ready for review October 13, 2026 14:33
@svmicano
svmicano requested a review from a team as a code owner October 13, 2026 14:33
Copilot AI balanced review requested due to automatic review settings October 13, 2026 14:33
@a0x1ab

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A successful MI database creation can leak a billable resource when an immediate output check fails before cleanup is enabled.

1 open finding
What changed in this PR

Adds SQL short-term backup retention immutability support for SQL Database and Managed Instance policies.

Changes:

  • Adds locking, confirmation, status output, and preview API support.
  • Introduces shared AAZ HTTP/LRO handling with final policy reads.
  • Adds comprehensive unit, scenario, safety, and playback coverage.
File Description
sql/​_help.py Documents immutability workflows.
sql/​_params.py Defines locking and confirmation arguments.
sql/​_str_policy.py Implements shared STR HTTP operations.
sql/​commands.py Registers custom STR handlers.
sql/​custom.py Adds validation and confirmation orchestration.
tests/​latest/​README.md Documents recording and safety procedures.
recordings/​test_sql_db_existing_policy_immutability.yaml Records existing-policy locking.
recordings/​test_sql_db_short_term_retention.yaml Records standard SQL DB retention.
recordings/​test_sql_db_short_term_retention_immutability.yaml Records SQL DB locking.
recordings/​test_sql_managed_db_short_retention.yaml Records standard MI retention.
recordings/​test_sql_managed_db_short_retention_immutability.yaml Records MI locking.
tests/​latest/​test_sql_commands.py Removes superseded scenarios.
tests/​latest/​test_sql_db_str_policy.py Adds SQL DB scenarios and safeguards.
tests/​latest/​test_sql_mi_str_policy.py Adds MI scenarios and safeguards.
tests/​latest/​test_sql_str_policy.py Adds offline HTTP and CLI tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +135 to +137
self.cmd('sql midb create --subscription {subscription} -g {rg} --mi {mi} -n {db}', checks=[
self.check('name', '{db}'), self.check('status', 'Online')])
self.database_created = True
@teresaritorto Teresa Ritorto (teresaritorto) added the Request X Engineering Agent Ask the agent to triage this issue or review this PR label Oct 15, 2026
@microsoft-github-policy-service

Copy link
Copy Markdown
Contributor

🔔 Routing this PR to @Azure/act-observability-squad.

@microsoft-github-policy-service

Copy link
Copy Markdown
Contributor

🔔 Routing this PR to @Azure/act-codegen-extensibility-squad.

@JaysonTaiMicrosoft

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

)


class _ShortTermRetentionPolicy(AAZCommand):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

svmicano

  1. Please generate this with AAZ, this should generally end up in a /aaz sub directory. You can refer to the following for more information https://azure.github.io/aaz-dev-tools/
  2. Provide the appropriate upstream PR after completing step 1

@x-engineering-agent

Copy link
Copy Markdown
Contributor

X Engineering Agent Report

⏳ In progress — X Engineering Agent is reviewing this PR at 93e16b8. This comment will be updated with the results.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants